Skip to content

feat(cmd): add side-aware targeting and trends telemetry#35

Merged
omarshahine merged 2 commits intosteipete:mainfrom
igormf:feat/side-aware-targeting-pr
Apr 18, 2026
Merged

feat(cmd): add side-aware targeting and trends telemetry#35
omarshahine merged 2 commits intosteipete:mainfrom
igormf:feat/side-aware-targeting-pr

Conversation

@igormf
Copy link
Copy Markdown
Contributor

@igormf igormf commented Apr 17, 2026

Summary

This is the follow-up feature PR requested in #30 after #24 landed.

It keeps the auth/runtime fixes that merged through #24 on main and layers the user-facing targeting improvements on top of the current codebase.

What Changed

  • add side-aware targeting helpers so commands can address left, right, all, or an explicit household user
  • make status report discovered household targeting mode, including split sides vs solo
  • make on, off, and temp default to all discovered targets when no side/user is provided
  • keep per-user control paths for households that expose individual users instead of simple side labels
  • add timezone-aware trends/presence support and tests around the new targeting logic
  • preserve the token cache/runtime compatibility needed for local Windows execution

UX Notes

Examples supported by this branch:

  • eightctl status
  • eightctl on
  • eightctl off --side right
  • eightctl temp -- -40
  • eightctl temp --side left 20
  • eightctl presence --from 2026-04-01 --to 2026-04-07

If the household is split, side names are surfaced. If the device is effectively single-zone, commands report solo and target the single available zone.

Verification

  • go test ./...
  • golangci-lint run ./...

Context

@omarshahine
Copy link
Copy Markdown
Collaborator

Thanks. I'll take a look this weekend.

@omarshahine omarshahine merged commit dd60677 into steipete:main Apr 18, 2026
1 check passed
@omarshahine
Copy link
Copy Markdown
Collaborator

omarshahine commented Apr 18, 2026

Merged — thanks for the thorough work here, @igormf! The side-aware targeting and --all-sides status read cleanly against the live API, and the new tests were a nice touch.

A few small follow-ups I'll handle in separate PRs so this can land now:

  1. Side resolution via user.currentDevice.side breaks when the household is in Away mode (the API returns side="away" for every user, so --side left errors out with available sides: away). Plan: resolve side from the device payload's leftUserId / rightUserId that you already fetch in HouseholdUserTargets.
  2. resolveAPITimezone falls back to UTC when time.Local.String() == "Local". We'll port @dtrinh's /etc/localtime / $TZ resolution from fix: resolve local timezone to IANA name for sleep commands #21 so the API gets the real IANA zone.
  3. Unrelated to this PR, but surfaced while testing: the tokencache picks the first keychain backend whose Open() succeeds, even when Set() then fails (e.g., macOS Keychain without a writable login keychain). Should fall through to FileBackend on write failure — separate PR.

Appreciate the clean split-out from #30 and the test coverage.

omarshahine added a commit to omarshahine/eightctl that referenced this pull request Apr 18, 2026
Three independent bugs surfaced while testing steipete#35:

1. HouseholdUserTargets relied on `user.currentDevice.side`, which the
   Eight Sleep API overwrites with the sentinel "away" for every user
   when the household is in Away mode. `--side left|right` therefore
   errored with `available sides: away` for any Away-mode household.
   Resolve side from the `/devices` payload instead: when Away mode
   blanks the top-level `leftUserId`/`rightUserId`, the real IDs live
   inside `awaySides` as `{"leftUserId":"…","rightUserId":"…"}`. Also
   filter the "available sides" error list to drop the Away sentinel.

2. `resolveAPITimezone` fell back to UTC whenever `time.Local.String()`
   returned "Local". Port the `/etc/localtime` / `/etc/timezone` / `$TZ`
   IANA resolution from PR steipete#21 (@dtrinh) so `presence`, `sleep day`,
   `sleep range`, and `metrics` get the real local zone the API expects.

3. Tokencache picked the first backend whose `Open()` succeeded even
   when every `Set()` then failed with `Keychain Error (-61)` — e.g.,
   macOS sessions without a writable login keychain. `Save`, `Load`,
   and `Clear` now fall through to a FileBackend-only keyring so the
   OAuth token survives across invocations instead of re-running the
   password grant on every command and tripping Eight Sleep's auth
   rate limiter.

Co-Authored-By: Danny Trinh <noreply@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
omarshahine added a commit that referenced this pull request Apr 18, 2026
…ck (#36)

* fix: away-mode side resolution, IANA timezone lookup, keychain fallback

Three independent bugs surfaced while testing #35:

1. HouseholdUserTargets relied on `user.currentDevice.side`, which the
   Eight Sleep API overwrites with the sentinel "away" for every user
   when the household is in Away mode. `--side left|right` therefore
   errored with `available sides: away` for any Away-mode household.
   Resolve side from the `/devices` payload instead: when Away mode
   blanks the top-level `leftUserId`/`rightUserId`, the real IDs live
   inside `awaySides` as `{"leftUserId":"…","rightUserId":"…"}`. Also
   filter the "available sides" error list to drop the Away sentinel.

2. `resolveAPITimezone` fell back to UTC whenever `time.Local.String()`
   returned "Local". Port the `/etc/localtime` / `/etc/timezone` / `$TZ`
   IANA resolution from PR #21 (@dtrinh) so `presence`, `sleep day`,
   `sleep range`, and `metrics` get the real local zone the API expects.

3. Tokencache picked the first backend whose `Open()` succeeded even
   when every `Set()` then failed with `Keychain Error (-61)` — e.g.,
   macOS sessions without a writable login keychain. `Save`, `Load`,
   and `Clear` now fall through to a FileBackend-only keyring so the
   OAuth token survives across invocations instead of re-running the
   password grant on every command and tripping Eight Sleep's auth
   rate limiter.

Co-Authored-By: Danny Trinh <noreply@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* style: apply gofumpt formatting to test files

CI's gofumpt format check caught whitespace drift in the two new test
files. No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: isolate test helpers from the default file keyring

The fallback path added in the previous commit made `tokencache.Load`
reach into `~/.config/eightctl/keyring/` whenever the primary backend
returned ErrKeyNotFound. Existing test helpers (`useTempKeyring` in
cmd, `withTestKeyring` in tokencache) only overrode `openKeyring`,
so tests that exercised the miss path could see the real filesystem
and return stale state — this is what caused
`TestRequireAuthFieldsFailsWithoutCacheOrCreds` to fail in CI.

Both helpers now override `openKeyring` and `openFileKeyring` with the
same tmp-backed opener, keeping the fallback entirely in-process.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* style: address self-review nits

- Move the IANA-discovery doc comment onto `defaultLocalIANA` where
  the behavior lives; leave a one-liner on the swappable var.
- Make `unwritableKeyring.Remove` return the same sentinel as `Set`
  so the fake behaves consistently if a future test exercises it.

No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Omar Shahine <10343873+omarshahine@users.noreply.github.com>
Co-authored-by: Danny Trinh <noreply@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants